Represent big integers as arrays of bytes #87
Conversation
Signed-off-by: Moritz Barsnick <[email protected]>
Signed-off-by: Moritz Barsnick <[email protected]>
…tet array Signed-off-by: Moritz Barsnick <[email protected]>
…tet array Signed-off-by: Moritz Barsnick <[email protected]>
For code readability. Signed-off-by: Moritz Barsnick <[email protected]>
When adding the final byte or octet, skip it when its value is zero. Trailing 0-octets are meaningless in both cases and shouldn't be provided. Signed-off-by: Moritz Barsnick <[email protected]>
Found by review, confirmed by cppcheck. Also reorder the code lines to a more logical order. Signed-off-by: Moritz Barsnick <[email protected]>
Signed-off-by: Moritz Barsnick <[email protected]>
Signed-off-by: Moritz Barsnick <[email protected]>
|
Missing:
|
barsnick
changed the title
| } | ||
| if (dummy_count > 0) { | ||
| *(current_octet-1) |= EXI_BASETYPES_OCTET_SEQ_FLAG_MASK; | ||
| if (dummy_count > 0 && dummy != 0) { |
There was a problem hiding this comment.
Is dummy != 0 here necessary?
It's relevant, I just read dummy_count twice...
|
libcbv2g with the generated code builds without problems. Unit tests passed and EVerest SIL work as well. So happy to merge this one 👍 |
| @@ -134,6 +134,24 @@ int exi_basetypes_convert_64_from_signed(const exi_signed_t* exi_signed, int64_t | |||
| return res; | |||
| } | |||
|
|
|||
| static void _reverse_array(uint8_t* data, size_t data_size) | |||
| { | |||
| if (!data || !data_size) | |||
There was a problem hiding this comment.
If this function is static and you control the inputs, why should it have such a silent error handling of data? It will have gone wrong in exi_basetypes_convert_bytes_from_unsigned before this function is reached.
data_size makes sense of course.
| @@ -134,6 +134,24 @@ int exi_basetypes_convert_64_from_signed(const exi_signed_t* exi_signed, int64_t | |||
| return res; | |||
| } | |||
|
|
|||
| static void _reverse_array(uint8_t* data, size_t data_size) | |||
There was a problem hiding this comment.
I get why we want to reverse it, but I rather see it written backwards from the get go to speed it up a bit.
Nice to have. I realise this is technically not easy.
|
Another general question then: do you want to change the generated bigint datatype too? You could just use the uint8_t array, a sign and a size struct instead of using the exi_signed thingy for instance. I initially picked the latter out of ease, but do tell what you think is better. You can mess with alignment and pack the struct a bit more by decreasing the unneeded bits for the size; struct {
uint8_t data[20];
uint8_t size : 7;
uint8_t sign : 1;
} SerialNumber;I will cost some computational overhead though. Probably not worth it. |
Describe your changes
In PR #65, support for proper coding of large integer (xs:integer) was added, aiding in the proper handling of X509SerialNumber.
Yet the API presented those numbers in the same style as EXI internally, i.e. with 8/7 padding, adding an indicator MSB to each octet for understanding whether the sequence continues.
This change now expects those numbers as arrays of bytes (still within an
exi_unsigned_t), representing the integer, without any padding. The byte order is big-endian, i.e. leading bytes in the array have the most significant meaning.Issue ticket number and link
Relates to issue #54
Amends PR #65
Checklist before requesting a review